Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOC-4464 examples for llen, lpop, lpush, lrange, rpop, and rpush #3234

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

andy-stark-redis
Copy link
Contributor

@andy-stark-redis andy-stark-redis commented Jan 15, 2025

DOC-4464, DOC-4469, DOC-4474, DOC-4479, DOC-4489, and DOC-4484.

Go versions of the examples for LLEN, LPOP, LPUSH, LRANGE, RPOP, and RPUSH.

@andy-stark-redis andy-stark-redis self-assigned this Jan 15, 2025
@andy-stark-redis andy-stark-redis marked this pull request as ready for review January 15, 2025 13:54
@ndyakov ndyakov self-requested a review February 7, 2025 08:55
Copy link
Collaborator

@ndyakov ndyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use better variable names for the examples.

Comment on lines +28 to +44
lLenResult1, err := rdb.LPush(ctx, "mylist", "World").Result()

if err != nil {
panic(err)
}

fmt.Println(lLenResult1) // >>> 1

lLenResult2, err := rdb.LPush(ctx, "mylist", "Hello").Result()

if err != nil {
panic(err)
}

fmt.Println(lLenResult2) // >>> 2

lLenResult3, err := rdb.LLen(ctx, "mylist").Result()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change and use better variable names? For example, when the result is from LPush, if you would like to follow the same naming you do in other examples, makes sense the variable to be named lPushResult

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change and use better variable names? For example, when the result is from LPush, if you would like to follow the same naming you do in other examples, makes sense the variable to be named lPushResult

@ndyakov As with the set commands, the result variable name comes from the name of the command that the example is for (in this case, all the code in the example is for the LLEN page, so the vars are lLenResultxx). With the LPUSH example, it's just a coincidence that the first command in the code is an lpush(...), which is why the var name relates to the command there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see... Maybe we can use other naming convention? At least for me it was misleading having lLenResult2 as a result of LPush.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants